Report array&callable pass as wrong param#5573
Report array&callable pass as wrong param#5573phpstan-bot wants to merge 6 commits intophpstan:2.1.xfrom
array&callable pass as wrong param#5573Conversation
… intersections in `MissingTypehintCheck` - In `MissingTypehintCheck::getIterableTypesWithMissingValueTypehint()`, the special-case block for `array&callable` intersection types was skipping the array part entirely, never checking it for missing value types - Added a check for the array inner type's iterable value type before skipping it, so `callable-array`, `callable&array`, and `array&callable(...)` now correctly report `missingType.iterableValue` when the array has no value type - Updated test expectations for `doIntersection()` (gains a third error for the outer array) and `doFoo()` (now reports missing iterable value type) - Added new test case `doBaz()` with explicit `callable&array` parameter - All consumers of `MissingTypehintCheck` (method/function parameters, return types, properties, constants, @var tags, @phpstan-assert) are automatically fixed since they share the same check
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
No, this behavior is on purpose.
We don't want to report missingTypehint check for array&callable since we can infer it.
But we should still report passing array&callable when passed to a function expecting array.
For instance we already report passing array&callable to array{string, string} (because it could be array{object|string, string}).
…ngTypehintCheck The missing typehint suppression for array&callable is intentional because PHPStan can infer the value type (object|non-falsy-string). Reverts the MissingTypehintCheck changes and their test expectations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…rray&callable When an array&callable intersection is checked for acceptance, the raw ArrayType(mixed, mixed) component was used, causing array<int> to incorrectly accept array&callable. Now the array component uses the narrowed key/value types (int<0,1>, object|non-falsy-string) so the acceptance check correctly rejects incompatible array types. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
VincentLanglet
changed the title
array&callable intersections in MissingTypehintCheckarray&callable pass as wrong param
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
Use another file bug-14549-bis.php for your tests
And add test with callable&array passed to a method expecting constant array
- array{string, string} => Error
- array{object|string, string} => No error
- array{object|string, string, string} => Error
…d constant array test cases
Test callable&array passed to methods expecting constant array types:
- array{string, string} => Error (object at index 0 is not string)
- array{object|string, string} => No error (matches callable-array shape)
- array{object|string, string, string} => Error (callable-array has 2 elements, not 3)
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Done. Here's what I changed:
|
VincentLanglet
left a comment
VincentLanglet
left a comment
There was a problem hiding this comment.
I can reproduce a similar issue with array<mixed>&hasOffsetValue(1,1) which seems acceptedBy array<string>.
Maybe IntersectionType::isAccepted implementation is wrong.
I wonder if lazyMaxMin is the right check
When looking for A&B isAcceptedBy C
If C does not accept A, A&B cannot be accepted.
What about a strategy
- If a NO => Return No
- Else returns the Max (Maybe & Yes)
What would be the impact of such change ?
|
I processed this review but have nothing to report. |
…d of special-casing callable&array Replace the callable&array-specific narrowing logic with a general approach: after the standard lazyMaxMin check, if it returns Yes, verify with isSuperTypeOf that the accepting type is actually a supertype of the full intersection. This catches cases where MixedType's accepts-everything behavior causes false acceptances (e.g. array<int> falsely accepting array<mixed>&hasOffsetValue or array<mixed>&callable intersections). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
Done. Here's a summary of what I changed: The problem: The reviewer pointed out that the PR's approach of special-casing Root cause: The fix: Replace the This approach:
|
| /** | ||
| * @param callable&array $task | ||
| */ | ||
| public function doBaz(array $task): void | ||
| { | ||
| } |
There was a problem hiding this comment.
Done. Removed the doBaz() method from bug-14549.php and its corresponding test expectation in MissingMethodParameterTypehintRuleTest.php. All 11,971 tests pass and make phpstan reports no errors.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
This seems to work but I wonder if I and the bot are not making a mistake by confusing accepts and isSuperTypeOf... And there is multiple integration test failure I'll need to check I'll need to take more time on it, just wanted to know if it's inspire you @staabm ? |
2 participants
Summary
When a parameter, return type, or property was declared with an
array&callableintersection type (including thecallable-arrayshorthand),MissingTypehintCheckdid not report the missing iterable value type for the array part. This meant users were not warned about the imprecisearray<mixed>in these intersections, and downstream argument type checks couldn't catch mismatches (e.g., passingarray&callabletoarray<int>).Changes
src/Rules/MissingTypehintCheck.php: In the special-case block forarray&callableintersections (lines 76–96), added a check on each array inner type'sgetIterableValueType(). If the value type is implicitmixed, it is now added to the missing-typehint list. The non-array parts (callable) are still traversed for their own inner type checks (e.g., callable parameter/return arrays).doBaz()with@param callable&arrayintests/PHPStan/Rules/Methods/data/bug-14549.phptests/PHPStan/Rules/Methods/MissingMethodParameterTypehintRuleTest.php:doFoo()(callable-array) now reportsmissingType.iterableValuefor its array partdoIntersection()(array&callable(array): array) now reports a thirdmissingType.iterableValuefor the outer arraydoBaz()(callable&array) reports bothmissingType.iterableValueandmissingType.callableRoot cause
MissingTypehintCheck::getIterableTypesWithMissingValueTypehint()had a special-case block (lines 76–92) for intersection types that are both callable and array. This block extracted the non-array parts and traversed only those, completely skipping the array parts. The array parts were never checked for missing value types, socallable-array/callable&arraysilently passed the check even though their array part has implicitmixedvalues.The fix adds the missing value type check for each array inner type within the existing special-case block, before the
continuethat skips it.Analogous cases probed
Since the fix is in the shared
MissingTypehintCheckclass, all consumers are automatically fixed:MissingMethodParameterTypehintRule) — tested and verifiedMissingFunctionParameterTypehintRule) — shares the same check, tests passMissingMethodReturnTypehintRule) — shares the same check, tests passMissingFunctionReturnTypehintRule) — shares the same check, tests passMissingPropertyTypehintRule) — shares the same check, tests passMissingClassConstantTypehintRule) — shares the same check, tests passgetIterableTypesWithMissingValueTypehint()methodNon-array callable intersections (e.g.,
Traversable&callable) don't hit this code path (the condition requiresisArray()->yes()), and they are already handled correctly by the general iterable check below.Test
testBug14549inMissingMethodParameterTypehintRuleTestverifies:callable-arrayreports missing iterable value typearray&callable(array): arrayreports missing value type for all three arrays (outer + callable param + callable return)callable&arrayreports both missing iterable value type and missing callable signatureFixes phpstan/phpstan#14549